Conversation
|
@justsmth It seems the ruby/rubygems repository doesn't have the GitHub Actions CI case for Ruby with ruby/openssl built with AWS-LC right now. Perhaps, adding the AWS-LC latest stable version case to GitHub Actions to this repository may be helpful if we consider AWS-LC case. I haven't checked if this PR's modified tests pass for RubyGems with Ruby OpenSSL built with AWS-LC. As a reference, ruby/openssl repository has the AWS-LC latest stable version case on the CI. There is a trade-off about adding the AWS-LC case to the CI. Pros:
Cons:
I am not a maintainer of the ruby/rubygems. So, consider my opinion as assumption which may not happen. If you work on this before maintainers agree the direction, you may waste your time. The steps can be as follows.
The candidate GitHub Actions yml file to add this case is Until ruby/rubygems maintainers comment on this PR, I don't have anything to comment on this PR. I pend my approval for this PR for now, while I saw you requested a review from me using the PR's function. Let's wait for the maintainer's reviews. |
|
About the AWS-LC job, I am ok to add it. But I do not want to build AWS-LC on every run. If we can cache the build like your step 1, the job is worth it. @junaruga For the change itself, could you review this? |
|
@hsbt All right. Yes, I can review the change. @justsmth So, let's add AWS-LC case to this repository's CI on this PR. I feel that adding new job such as "rubygems-openssl" in .github/workflows/rubygems.yml makes sense. Right now, the ruby/openssl CI doesn't cache the compiled AWS-LC. However, it's possible to cache the compiled AWS-LC there. So, I just sent the PR ruby/openssl#1108 to the ruby/openssl now to cache the compiled AWS-LC. You can refer to the https://github.com/ruby/openssl/blob/master/.github/workflows/test.yml with the PR. |
@justsmth Note I sent the PR ruby/openssl#1109, as a follow up of ruby/openssl#1108. You can refer to the ruby/openssl#1109 about how to add the cached compiled aws-lc case to this repository. |
@justsmth Note I sent the PR ruby/openssl#1110 as a follow up of ruby/openssl#1109. We misunderstood the cache validity of the actions/cache in ruby/openssl. So, we are adding the 1 week (7 days) cache validity by the PR. You can refer to the PR. |
RubyGems currently assumes an SSL library either supports all ML-DSA operations or none of them. It also identifies ML-DSA keys using the provider-specific algorithm names registered by OpenSSL 3.5. AWS-LC loads ML-DSA keys and certificates and signs and verifies with them, but it does not provide key generation by algorithm name and registers 2.16.840.1.101.3.4.3.18 as "MLDSA65" instead of "ML-DSA-65". Match the SubjectPublicKeyInfo OIDs defined by RFC 9881 so key identification does not depend on provider-specific names. The existing support_ml_dsa_key? probe tests key generation, but several tests only load checked-in keys and certificates. Add a separate key-loading probe for fixture-based tests while keeping generation tests on the original probe.
Build and cache the latest stable AWS-LC weekly, rebuild Ruby OpenSSL against it, and run the RubyGems test suite. Include the new job in the aggregate all-pass gate.
d0217e9 to
73aa440
Compare
|
Thanks for the feedback. I rebased onto current master and added the requested cached AWS-LC CI job as its own commit. The CI workflows are currently awaiting approval to run. |
| git clone https://github.com/aws/aws-lc.git . | ||
| AWS_LC_RELEASE=$(git tag --sort=-creatordate --list "v*" | head -1) | ||
| git checkout "$AWS_LC_RELEASE" | ||
| cmake -DCMAKE_INSTALL_PREFIX="$HOME/openssl" -DCMAKE_INSTALL_LIBDIR=lib |
There was a problem hiding this comment.
I want to replace the cmake command with the following command.
cmake -DCMAKE_INSTALL_PREFIX="$HOME/openssl" -DCMAKE_INSTALL_LIBDIR=lib -DBUILD_SHARED_LIBS=1 -DCMAKE_INSTALL_RPATH="$HOME/openssl"
Because while I understand the -DCMAKE_INSTALL_PREFIX="$HOME/openssl" -DCMAKE_INSTALL_LIBDIR=lib is minimal configure options to build in GitHub Actions Ubuntu, I think they can be documentation for people refer to rubygems.yml's cmake and make commands to compile AWS-LC on their local environment.
I needed -DBUILD_SHARED_LIBS=1 to create shared objects on my local environment Fedora Linux.
I also needed rpath, -DCMAKE_INSTALL_RPATH="$HOME/openssl" to run openssl command on local. Though I understand running openssl command is not directly related to CI.
$ ~/.local/aws-lc-5.8.0/bin/openssl version
OpenSSL 1.1.1 (compatible; AWS-LC 5.8.0)
Without the rpath, the openssl command fails like this.
$ ~/.local/aws-lc-5.8.0/bin/openssl version
/home/jaruga/.local/aws-lc-5.8.0/bin/openssl: symbol lookup error: /home/jaruga/.local/aws-lc-5.8.0/bin/openssl: undefined symbol: OPENSSL_free
There was a problem hiding this comment.
Using CI yml file as a document to show to users may be controversial.
Because ruby/openssl's CI doesn't use -DBUILD_SHARED_LIBS=1 and -DCMAKE_INSTALL_RPATH="$HOME/openssl" to compile AWS-LC.
cmake -DCMAKE_INSTALL_PREFIX=$HOME/openssl -DCMAKE_INSTALL_LIBDIR=lib
make -j4 && make install
ruby/openssl has a document to build OpenSSL with rpath configure option.
https://github.com/ruby/openssl/blob/master/CONTRIBUTING.md#with-different-versions-of-openssl
$ ./Configure \
--prefix=$OPENSSL_DIR \
--libdir=lib \
enable-fips \
enable-trace \
'-Wl,-rpath,$(LIBRPATH)' \
-O0 -g3 -ggdb3 -gdwarf-5
I think adding -DBUILD_SHARED_LIBS=1 and -DCMAKE_INSTALL_RPATH="$HOME/openssl" here is convenient as a user in my humble opinion. But not sure in term of a minimal logic to needed to run tests.
There was a problem hiding this comment.
I think adding -DBUILD_SHARED_LIBS=1 and -DCMAKE_INSTALL_RPATH="$HOME/openssl" here is convenient as a user in my humble opinion. But not sure in term of a minimal logic to needed to run tests.
Sorry I thought again. We shouldn't add the -DBUILD_SHARED_LIBS=1 and -DCMAKE_INSTALL_RPATH="$HOME/openssl" here. These can be documented in AWS-LC repository not in this repository. Sorry for the noise.
| ruby --disable-gems -ropenssl -e ' | ||
| abort "Ruby OpenSSL did not load AWS-LC" unless OpenSSL::OPENSSL_VERSION.include?("AWS-LC") | ||
| OpenSSL::PKey.read(File.binread("test/rubygems/mldsa65_private_key.pem")) | ||
| puts "Ruby OpenSSL #{OpenSSL::VERSION}: #{OpenSSL::OPENSSL_VERSION}" |
There was a problem hiding this comment.
I think this step is useful to make sure if the AWS-LC is loaded or default OpenSSL is wrongly loaded for someone reasons. I also think printing AWS-LC version is useful, because we skip "Compile AWS-LC" step when the compiled AWS-LC is cached. Therefore we don't see the used AWS-LC version in CI log. However, I suspect if we need the line OpenSSL::PKey.read(File.binread("test/rubygems/mldsa65_private_key.pem")), because the unit tests check it. I feel the ML-DSA loading test by OpenSSL::PKey.read here is too detailed.
There was a problem hiding this comment.
I think putting --disable-gems is wrong in this context. Because we want to make sure that the later step bin/rake test uses AWS-LC where --disable-gems is not set.
|
@justsmth I reviewed your PR. Please check my comments in conversations. And check my comment below. For the following part of the commit message:
This means AWS-LC doesn't support However, AWS-LC support loading ML-DSA. That's why So, I think we should use So, here is my patch. I confirmed the CI passed with this patch in my fork repository. The CI log is here. You can apply and squash it with your main commit. It's okay to squash multiple commits to 1 commit including CI fix. Because it's easy to rebase the PR. From dd645a619e44868bbac73e81ccb52442a6648ec2 Mon Sep 17 00:00:00 2001
From: Jun Aruga <jaruga@redhat.com>
Date: Mon, 14 Sep 2026 19:52:32 +0200
Subject: [PATCH] Gate ML-DSA tests and probes on the capability they need
Assisted-by: Claude:Opus 4.6
---
test/rubygems/pqc_utilities.rb | 7 ++---
.../test_gem_commands_cert_command.rb | 1 +
test/rubygems/test_gem_security.rb | 26 +++++++++----------
3 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/test/rubygems/pqc_utilities.rb b/test/rubygems/pqc_utilities.rb
index 0eb6ccf73d..188ee890cd 100644
--- a/test/rubygems/pqc_utilities.rb
+++ b/test/rubygems/pqc_utilities.rb
@@ -85,7 +85,9 @@ def self.support_ml_dsa_cert?
@support_ml_dsa_cert =
begin
- key = OpenSSL::PKey.generate_key("ML-DSA-65")
+ key = OpenSSL::PKey.read(
+ File.read(File.join(CERTS_DIR, "mldsa65_private_key.pem"))
+ )
cert = OpenSSL::X509::Certificate.new
cert.subject = cert.issuer = OpenSSL::X509::Name.new([["CN", "probe"]])
cert.public_key = OpenSSL::PKey.read(key.public_to_pem)
@@ -93,10 +95,9 @@ def self.support_ml_dsa_cert?
cert.not_after = Time.now + 60
cert.sign(key, nil)
true
- # NoMethodError: JRuby's Ruby OpenSSL lacks generate_key.
# TypeError: Ruby OpenSSL < 3.3 rejects a nil digest here.
rescue OpenSSL::PKey::PKeyError, OpenSSL::X509::CertificateError,
- NoMethodError, TypeError
+ TypeError
false
end
end
diff --git a/test/rubygems/test_gem_commands_cert_command.rb b/test/rubygems/test_gem_commands_cert_command.rb
index d678c97785..d30c4ccc91 100644
--- a/test/rubygems/test_gem_commands_cert_command.rb
+++ b/test/rubygems/test_gem_commands_cert_command.rb
@@ -158,6 +158,7 @@ def test_execute_build_key_algorithm_ec_key
end
def test_execute_build_key_algorithm_ml_dsa_65_key
+ omit_unless_support_ml_dsa_key
omit_unless_support_ml_dsa_cert
passphrase = "Foo bar"
diff --git a/test/rubygems/test_gem_security.rb b/test/rubygems/test_gem_security.rb
index 28886cecb8..ebaac752a6 100644
--- a/test/rubygems/test_gem_security.rb
+++ b/test/rubygems/test_gem_security.rb
@@ -86,7 +86,7 @@ def test_class_create_cert_email
end
def test_class_create_cert_email_ml_dsa_65_without_cert_support
- omit_unless_support_ml_dsa_key
+ omit_unless_support_ml_dsa_key_load
omit_if_support_ml_dsa_cert
e = assert_raise Gem::Security::Exception do
@@ -219,9 +219,9 @@ def test_class_digest_required_raises_unsupported_algorithm
end
def test_class_digest_required_ml_dsa_65
- omit_unless_support_ml_dsa_key
+ omit_unless_support_ml_dsa_key_load
- refute Gem::Security.digest_required?(Gem::Security.create_key("ml-dsa-65"))
+ refute Gem::Security.digest_required?(ML_DSA_65_PRIVATE_KEY)
end
def test_class_get_public_key_rsa
@@ -237,7 +237,7 @@ def test_class_get_public_key_ec
end
def test_class_get_public_key_ml_dsa_65
- omit_unless_support_ml_dsa_key
+ omit_unless_support_ml_dsa_key_load
pkey = Gem::Security.get_public_key(ML_DSA_65_PRIVATE_KEY)
@@ -360,7 +360,7 @@ def test_class_trust_dir
end
def test_class_write_private_key
- key = Gem::Security.create_key "rsa"
+ key = PRIVATE_KEY
path = File.join @tempdir, "test-private_key.pem"
@@ -374,9 +374,9 @@ def test_class_write_private_key
end
def test_class_write_private_key_ml_dsa_65
- omit_unless_support_ml_dsa_key
+ omit_unless_support_ml_dsa_key_load
- key = Gem::Security.create_key "ml-dsa-65"
+ key = ML_DSA_65_PRIVATE_KEY
path = File.join @tempdir, "test-ml-dsa-private_key.pem"
@@ -390,7 +390,7 @@ def test_class_write_private_key_ml_dsa_65
end
def test_class_write_private_key_encrypted
- key = Gem::Security.create_key "rsa"
+ key = PRIVATE_KEY
path = File.join @tempdir, "test-private_encrypted_key.pem"
@@ -406,9 +406,9 @@ def test_class_write_private_key_encrypted
end
def test_class_write_private_key_encrypted_ml_dsa_65
- omit_unless_support_ml_dsa_key
+ omit_unless_support_ml_dsa_key_load
- key = Gem::Security.create_key "ml-dsa-65"
+ key = ML_DSA_65_PRIVATE_KEY
path = File.join @tempdir, "test-ml-dsa-private_encrypted_key.pem"
@@ -424,7 +424,7 @@ def test_class_write_private_key_encrypted_ml_dsa_65
end
def test_class_write_private_key_encrypted_cipher
- key = Gem::Security.create_key "rsa"
+ key = PRIVATE_KEY
path = File.join @tempdir, "test-private_encrypted__with_non_default_cipher_key.pem"
@@ -445,9 +445,9 @@ def test_class_write_private_key_encrypted_cipher
end
def test_class_write_private_key_encrypted_cipher_ml_dsa_65
- omit_unless_support_ml_dsa_key
+ omit_unless_support_ml_dsa_key_load
- key = Gem::Security.create_key "ml-dsa-65"
+ key = ML_DSA_65_PRIVATE_KEY
path = File.join @tempdir,
"test-ml-dsa-private_encrypted_with_non_default_cipher_key.pem"
--
2.53.0With this patch, there are only 10 omissions on AWS-LC case. https://github.com/junaruga/rubygems/actions/runs/34886027493/job/104116786972#step:10:20 |
|
How do we confirm that the AWS-LC registers 2.16.840.1.101.3.4.3.18 as "MLDSA65"? Do you have a link to refer to, mentioning the term "MLDSA65"? |
|
I think the current 2nd commit need line breaks within 80 character line length. The one line is too long. |
3 participants
What was the end-user or developer problem that led to this PR?
RubyGems currently assumes an SSL library either supports all ML-DSA operations or none of them:
Gem::Security.ml_dsa_key?identifies ML-DSA using OpenSSL 3.5 algorithm names such asML-DSA-65. Those names are library-specific; AWS-LC registers the same standard OID asMLDSA65, so RubyGems rejects a valid ML-DSA key as unsupported and cannot verify ML-DSA signed gems.Gem::PQCUtilities.support_ml_dsa_key?probes key generation but gates tests that only load checked-in keys and certificates. AWS-LC loads, signs, and verifies with ML-DSA, but does not generate an ML-DSA key by algorithm name. This causes four incorrect test failures and skips ten positive tests that AWS-LC can run.What is your fix for the problem, implemented in this PR?
ml_dsa_key?now identifies ML-DSA using the three standard SubjectPublicKeyInfo OIDs rather than library-specific display names.The tests gain a separate
support_ml_dsa_key_load?probe. Four negative tests that assert loading failure, plus ten positive fixture-based tests, now use the load capability instead of the generation capability. Tests that callGem::Security.create_keykeep the existing generation gate.These changes are combined because they test each other: re-gating the positive tests without the OID fix exposes four failures in
digest_required?, while the OID fix alone would remain untested on libraries that can load but not generate ML-DSA keys.This continues 9a43535, which separated key generation from certificate signing but not key generation from key loading.
Testing
Using Ruby master built against AWS-LC 5.7.0:
masterThe net omission change reflects ten newly exercised positive tests and four negative tests now correctly omitted.
Gem::Security::Policy#check_dataverifies an ML-DSA signature and rejects a tampered payload with this change.Using OpenSSL 3.0.13, which has no ML-DSA support, all five affected test files produce identical results before and after. The full RubyGems test suite passes, and
bin/rubocopreports no offenses on the seven changed files.I did not have an OpenSSL 3.5 build available locally. On 3.5 both capability probes remain true, and the OIDs matched here are the same OIDs exposed by its existing ML-DSA keys.
Make sure the following tasks are checked